-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: updated docstring for nanoseconds function per doc guidelines #21065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes - an Examples section would be a nice touch
pandas/_libs/tslibs/timedeltas.pyx
Outdated
|
||
Returns | ||
------- | ||
int : Number of nanoseconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the type and description on separate lines. Make sure you indent the description and add a period at the end of it.
pandas/_libs/tslibs/timedeltas.pyx
Outdated
.components will return the shown components | ||
See Also | ||
-------- | ||
Timedelta.components : Return all attributes with assigned values (i.e. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a period at the end of the description here. This isn't enforced and is inconsistent in the docs but should eventually be standardized
pandas/_libs/tslibs/timedeltas.pyx
Outdated
|
||
.components will return the shown components | ||
See Also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a reference to the other component attributes
Codecov Report
@@ Coverage Diff @@
## master #21065 +/- ##
=======================================
Coverage 91.83% 91.83%
=======================================
Files 153 153
Lines 49495 49495
=======================================
Hits 45454 45454
Misses 4041 4041
Continue to review full report at Codecov.
|
Made the changes suggested...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - thanks!
So... what is the resolution path if the travis-ci build fails?
|
Typically things won't get merged if they have errors. You can look at the logs to see what happens - in this case your change is failing the LINT check: pandas/_libs/tslibs/timedeltas.pyx:804:80: E501 line too long (84 > 79 characters)
pandas/_libs/tslibs/timedeltas.pyx:818:11: W291 trailing whitespace You should get the same error locally if you run flake8 (see contributing guide for details) |
|
|
||
Returns | ||
------- | ||
int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be on the same line with a colon? @TomAugspurger @jorisvandenbossche
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine as is. It's typically
name : type
Explanation
but when the name is obvious (it'd just be nanoseconds
here), it can be omitted.
thanks @chalmerlowe |
Thanks for closing. My First PR on pandas. There was some back and forth comments on the formatting... I followed the examples in the docstrings guide: With a single return value
With multiple return values
|
…andas-dev#21065) (cherry picked from commit 9f40757)
git diff upstream/master -u -- "*.py" | flake8 --diff